Skip to content

Replace thread.sleep in sync exchange v2 with coroutine-based delay#8986

Open
CDRussell wants to merge 1 commit into
developfrom
feature/craig/fix/craig/sync_exchange_v2_remove_sleep
Open

Replace thread.sleep in sync exchange v2 with coroutine-based delay#8986
CDRussell wants to merge 1 commit into
developfrom
feature/craig/fix/craig/sync_exchange_v2_remove_sleep

Conversation

@CDRussell

@CDRussell CDRussell commented Jun 24, 2026

Copy link
Copy Markdown
Member

Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1215729137633076?focus=true

Description

Replaces two Thread.sleep(...) calls in the sync v2 retry-backoff loops with delay(...):

  • ThirdPartyCredentialManager.retryingOnRateLimit (used when extending the account with a 3party credential and the server rate-limits the call)
  • SyncAccountRepository.retryingOnTransientError (used during the 3party → ddg upgrade login)

Switching to delay required a suspend cascades a little to these:

  • ThirdPartyCredentialManager.create()
  • SyncAccountRepository.createThirdPartyCredential() and joinAccountFromThirdPartyRecoveryCode()
  • RecoveryCodeProvider.createThirdPartyCredentialIfNeeded()
  • A small refactor inside ExchangeV2Runner.shareRecoveryCodeAndAdvanceLocked so the recovery-code fetch+send runs inside the launched coroutine instead of synchronously before it.

Intentional behaviour changes

  • SM mutex no longer held during the recovery-code fetch+send. In shareRecoveryCodeAndAdvanceLocked, the network work now runs inside the launched coroutine and outside mutex.withLock, so retry backoff doesn't block the dispatcher thread. A short-circuit at the top of the launch handles the common cancel-before-run race; any work that proceeds past it is best-effort, and any orphaned server-side credential is cleaned up by the BE's 5-min TTL.

Steps to test this PR

  • Manual: sync with a 3rd party browser

UI changes

None.


Note

Medium Risk
Touches 3party credential minting and account-join upgrade flows plus v2 pairing host recovery-code timing; the runner mutex change can alter concurrency during pairing but is intentional to avoid blocking the SM.

Overview
Replaces Thread.sleep with delay in sync v2 retry backoff loops (ThirdPartyCredentialManager rate-limit and concurrent-modification retries, SyncAccountRepository.retryingOnTransientError during 3party→ddg upgrade). That drives a suspend cascade through 3party credential creation, joinAccountFromThirdPartyRecoveryCode, RecoveryCodeProvider.createThirdPartyCredentialIfNeeded, and the dispatcher’s v2 outcome mappers that call those APIs.

In ExchangeV2Runner.shareRecoveryCodeAndAdvanceLocked, host recovery-code provisioning and send now run inside the launched IO coroutine (with an early session == null exit) instead of synchronously before launch, so retry backoff and account/credential network work no longer run while the exchange state-machine mutex is held.

Affected unit tests are updated to use runTest for the new suspend entry points.

Reviewed by Cursor Bugbot for commit 68ef3a9. Configure here.

CDRussell commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from a7181fd to 63e932f Compare June 25, 2026 15:08
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from 63e932f to 15b9f51 Compare June 25, 2026 15:23
@CDRussell CDRussell changed the base branch from develop to graphite-base/8986 June 26, 2026 10:30
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from 15b9f51 to 00e6b25 Compare June 26, 2026 10:30
@CDRussell CDRussell changed the base branch from graphite-base/8986 to feature/craig/sync_exchange_v2_upgrade_flow_use_same_hashed_password June 26, 2026 10:31
@CDRussell CDRussell changed the base branch from feature/craig/sync_exchange_v2_upgrade_flow_use_same_hashed_password to graphite-base/8986 June 26, 2026 10:56
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch 2 times, most recently from f7f50f7 to cbdc0d4 Compare June 26, 2026 12:04
@CDRussell CDRussell force-pushed the graphite-base/8986 branch from 9adc18b to b0288d5 Compare June 26, 2026 12:04
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from cbdc0d4 to 6089722 Compare June 26, 2026 17:00
@CDRussell CDRussell force-pushed the graphite-base/8986 branch from b0288d5 to 8343770 Compare June 26, 2026 17:00
@graphite-app graphite-app Bot changed the base branch from graphite-base/8986 to develop June 26, 2026 17:00
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from 6089722 to 0cd4c09 Compare June 26, 2026 17:00
@CDRussell CDRussell force-pushed the feature/craig/fix/craig/sync_exchange_v2_remove_sleep branch from 0cd4c09 to 68ef3a9 Compare July 4, 2026 01:31
@CDRussell CDRussell marked this pull request as ready for review July 4, 2026 01:32

@cmonfortep cmonfortep left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants